Skip to content

Fixed Python memory leak in map lookup.#14583

Merged
haberman merged 1 commit intoprotocolbuffers:25.xfrom
haberman:cherry-pick-python-memory-leak
Nov 1, 2023
Merged

Fixed Python memory leak in map lookup.#14583
haberman merged 1 commit intoprotocolbuffers:25.xfrom
haberman:cherry-pick-python-memory-leak

Conversation

@haberman
Copy link
Member

@haberman haberman commented Nov 1, 2023

Previously we were allocating memory on the message's arena every time we performed a map[key] or map.get(key) operation. This is unnecessary, as the key's data is only needed ephemerally, for the duration of the lookup, and we can therefore alias the Python object's string data instead of copying it.

This required fixing a bug in the convert.c operation. Previously in the arena==NULL case, if the user passes a bytes object instead of a unicode string, the code would return a pointer to a temporary Python object that had already been freed, leading to use-after-free. I fixed this by referencing the bytes object's data directly, and using utf8_range to verify the UTF-8.

Fixes: #14571
PiperOrigin-RevId: 578563555

Previously we were allocating memory on the message's arena every time we performed a `map[key]` or `map.get(key)` operation.  This is unnecessary, as the key's data is only needed ephemerally, for the duration of the lookup, and we can therefore alias the Python object's string data instead of copying it.

This required fixing a bug in the convert.c operation.  Previously in the `arena==NULL` case, if the user passes a bytes object instead of a unicode string, the code would return a pointer to a temporary Python object that had already been freed, leading to use-after-free.  I fixed this by referencing the bytes object's data directly, and using utf8_range to verify the UTF-8.

Fixes: protocolbuffers#14571
PiperOrigin-RevId: 578563555
@haberman haberman requested a review from a team as a code owner November 1, 2023 17:37
@haberman haberman requested review from anandolee and removed request for a team November 1, 2023 17:37
@haberman haberman added the 🅰️ safe for tests Mark a commit as safe to run presubmits over label Nov 1, 2023
@github-actions github-actions bot removed the 🅰️ safe for tests Mark a commit as safe to run presubmits over label Nov 1, 2023
@haberman haberman merged commit 59a66af into protocolbuffers:25.x Nov 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments